Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[luci] Propagate qparam backward in onnx-fake quant model #14489

Merged

Conversation

jinevening
Copy link
Contributor

This propagates qparam backward in onnx-fake quant model.

ONE-DCO-1.0-Signed-off-by: Hyukjin Jeong [email protected]

@jinevening jinevening force-pushed the luci/prop_qparam_backward_onnx branch from 52da2a0 to 9aeb9b7 Compare December 20, 2024 07:37
This propagates qparam backward in onnx-fake quant model.

ONE-DCO-1.0-Signed-off-by: Hyukjin Jeong <[email protected]>
@jinevening jinevening force-pushed the luci/prop_qparam_backward_onnx branch from 9aeb9b7 to afdbc7c Compare December 20, 2024 07:38
Comment on lines -191 to -192
// Non-const input must have been quantized
assert(node->quantparam() != nullptr);
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This pass is now used for QuantizeOnnxFakeQuantModelPass, which makes this assumption invalid.

Q) Is it safe to remove the assert?
A) I think yes. The assert was just for checking the behavior of QuantizeWithMinMaxPass. Even though node->quantparam() == nullptr, overwrite_quantparam can work without any problem.

@jinevening
Copy link
Contributor Author

To reviewers: This PR is for Llama3.2 quantization.

Copy link
Contributor

@mhs4670go mhs4670go left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@jinevening jinevening merged commit 5c65065 into Samsung:master Dec 24, 2024
9 checks passed
@jinevening jinevening deleted the luci/prop_qparam_backward_onnx branch December 24, 2024 02:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants